Skip to content

BUG: lib.infer_dtype with mixed-freq Periods #41526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 18, 2021

Conversation

jbrockmendel
Copy link
Member

@jbrockmendel jbrockmendel commented May 17, 2021

  • closes #xxxx
  • tests added / passed
  • Ensure all linting tests pass, see here for how to run them
  • whatsnew entry

Also adds a convert_period option to lib.maybe_infer_objects

@@ -2343,14 +2367,6 @@ def maybe_convert_objects(ndarray[object] objects, bint try_float=False,
elif util.is_float_object(val):
floats[i] = complexes[i] = val
seen.float_ = True
elif util.is_datetime64_object(val):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to the rest of the PR, this chunk of code is just redundant with a chunk below

@jreback jreback added Period Period data type Dtype Conversions Unexpected or buggy dtype conversions labels May 17, 2021
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have any user facing changes?

cdef class PeriodValidator(TemporalValidator):
cdef inline bint is_value_typed(self, object value) except -1:
return is_period_object(value)
cdef bint is_period_array(ndarray[object] values):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you not keep the current PeriodValidator format itself? (e.g. just put this function in validate). this is breaking the pattern.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the same pattern we use for is_datetime_with_singletz_array (and will end up using for is_interval_array in an upcoming PR)

@jbrockmendel
Copy link
Member Author

does this have any user facing changes?

replace handles periods better

@jreback
Copy link
Contributor

jreback commented May 17, 2021

does this have any user facing changes?

replace handles periods better

kk can you add a note.

@jbrockmendel
Copy link
Member Author

whatsnew added + green

@jreback jreback added this to the 1.3 milestone May 18, 2021
@jreback jreback merged commit c1749ce into pandas-dev:master May 18, 2021
@jreback
Copy link
Contributor

jreback commented May 18, 2021

thanks

@jbrockmendel jbrockmendel deleted the ref-maybe_convert_objects-2 branch May 18, 2021 22:27
TLouf pushed a commit to TLouf/pandas that referenced this pull request Jun 1, 2021
JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dtype Conversions Unexpected or buggy dtype conversions Period Period data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants